Set helpers #isEqual, adds some test coverage #186
Conversation
There was a problem hiding this comment.
Hello @mrflip, just a few nitpicks but overall looks good to me.
CHANGELOG.md
Outdated
| @@ -1,5 +1,9 @@ | |||
| # Changelog | |||
|
|
|||
| ## 0.39.3 | |||
There was a problem hiding this comment.
Should be 0.40.0 since we are adding a feature. I also usually add (provisional) at the right so that people know until the version is actually released.
test/set.js
Outdated
| @@ -11,23 +11,93 @@ describe('Set functions', function() { | |||
| describe('#.intersection', function() { | |||
|
|
|||
| it('should properly compute the intersection of two sets.', function() { | |||
| var A = new Set([1, 2, 3]), | |||
| B = new Set([2, 3, 4]); | |||
| var Exemplar = new Set([2, 3, 1]); | |||
There was a problem hiding this comment.
Those variables should not be capitalized.
There was a problem hiding this comment.
I updated all, including the pre-existing ones, to all be lowerCameled and more than one character
test/set.js
Outdated
| var Disjoint = new Set([7, 8, 9]); | ||
| var Empty = new Set([]) | ||
| // | ||
| var WithItself = functions.intersection(Exemplar, Exemplar); |
test/set.js
Outdated
|
|
||
| var I = functions.intersection(A, B); | ||
| it('Makes no guarantees for element ordering.', function() { |
There was a problem hiding this comment.
Fixed. In an act of overkill, because the jest eslinter enforces lowercase titles, I added the mocha linter to package.json. It does not; but it did catch some cases where there were arrow functions, which mocha doesn't like. I can remove or file that commit under its own PR if you prefer.
mrflip
force-pushed
the
SetIsEqual
branch
2 times, most recently
from
638fbe1
to
5543bde
Compare
|
I believe I've caught all the test style violations. |
Addresses #172
Implementation is that two sets are
isEqualif they are the same object, or have the same size and B has every element of A.I did this by duplicating the entire
isSubsetmethod and changing the relevant line, rather than first testing for size equality and then delegating toisSubset-- saving operations at the cost of code complexity.Also have added some test coverage in there. They are largely documentary in that they cover edge cases unlikely to crop up in the current implementation, but which a naïve outsider might like reassurance about (eg that intersecting with an empty set makes an empty set, etc)
Once I've addressed your comments, I can do the same to everything else with isSubset and for other tests.